Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stratified report fix #315

Merged
merged 5 commits into from
Oct 15, 2024
Merged

Stratified report fix #315

merged 5 commits into from
Oct 15, 2024

Conversation

lmd59
Copy link
Contributor

@lmd59 lmd59 commented Oct 10, 2024

Summary

Addresses UnexpectedProperty issue in #311.

New behavior

For episode-based measures with stratifiers, (as was already done for patient-based), we now default the strataCode to be the id if the strata.code?.text does not exist

Code changes

  • DetailedResultsBuilder.ts - defaults strataCode to strata.id as secondary preferred value for episode results
  • MeasureReportBuilder.ts - slight efficiency improvement and added clarifying comment
  • Adds tests addressing episode+stratifier cases in DetailedResultsBuilder.test.ts an MeasureReportBuilder.test.ts

Testing guidance

  • npm run check
  • Pull measure/patient from ticket and test using fqm-execution details -m "path\to\measures\EpisodeMeasure.json" -p "path\to\patients\Episode_Patient.json" -o "path\to\output\Episode_Details.json" --vs-api-key {apiKey}

Copy link

github-actions bot commented Oct 10, 2024

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
85.42% (+0.07% 🔼)
2425/2839
🟡 Branches
73.18% (+0.29% 🔼)
2284/3121
🟢 Functions
87.68% (+0.2% 🔼)
434/495
🟢 Lines
85.72% (+0.07% 🔼)
2342/2732

Test suite run success

459 tests passing in 31 suites.

Report generated by 🧪jest coverage report action from a3c69d3

Copy link
Contributor

@elsaperelli elsaperelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm just one small inline comment! Also, could be worth adding something about the potential prescence of stratifierResults on episodeResults? Either in the Stratification Section or Measure With Observation Function Section?

test/unit/MeasureReportBuilder.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@elsaperelli elsaperelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

@hossenlopp hossenlopp merged commit 303107f into master Oct 15, 2024
6 checks passed
@hossenlopp hossenlopp deleted the stratified-report-fix branch October 15, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants